fix(registry): emit pyproject hints as standalone comments for tomlkit 0.15#536
fix(registry): emit pyproject hints as standalone comments for tomlkit 0.15#536mattmillerai wants to merge 2 commits into
Conversation
…t 0.15
tomlkit 0.15.1 made `Item.comment()` validate its argument, raising
ValueError("Comment cannot contain line breaks") on any `\n`. Two call
sites in config_parser passed multi-line strings, so every call to
`create_comfynode_config()` raised and `comfy node init` was broken at
runtime, not just in tests (9 tests red on main; tomlkit was unpinned so
CI resolved the newest release).
Emit each hint line as a standalone `tomlkit.comment()` instead, which
keeps the hints on their own lines and uncommentable in place. The
classifiers hint moves into `create_comfynode_config`, the only point
where it can be positioned under `license` and before the `[project.urls]`
sub-table. Rendered output is byte-identical to the pre-break version
apart from dropped trailing whitespace.
Also pin `tomlkit>=0.13,<0.16` to stop silent upstream breakage; the fix
is verified against 0.13.0, 0.13.2, 0.14.0 and 0.15.1.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 1 finding(s).
| Severity | Count |
|---|---|
| 🟢 Low | 1 |
Panel: 5/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), claude-opus-4-8-thinking-xhigh:edge-case (parse_error), kimi-k2.5:edge-case (empty)
f91cab6 accidentally carried a 287-line uv.lock regeneration that has nothing to do with the HTTP oversize fix. The drift is real but pre-existing: #490 added the `bench` optional-dependency extra (anthropic>=0.40) to pyproject.toml without relocking, so main's lock has been stale since it merged — `uv lock --check` fails on main today. Any bare `uv run` in the repo silently re-locks and re-dirties the file, which is how it got picked up here. Re-locking is not this PR's job, and carrying it here invites conflicts with the dependency PRs already in flight (#533/#535/#536). No CI job consumes uv.lock, so reverting to main's copy is behavior-neutral. Filed as a follow-up instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mment
Under a CRLF checkout (no .gitattributes + Windows core.autocrlf=true) the
hint literals contain \r\n. hint.strip("\n") left a leading \r that
splitlines() turned into an empty first element, emitting a bare # comment
above each hint. Use hint.strip(). Add a CRLF regression test.
Addresses cursor-review (gemini-3.1-pro edge-case) on PR #536.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ELI-5
comfy node initwrites a starterpyproject.tomlwith a couple of commented-out"here's what you could put here" hints. It built those hints by handing tomlkit a
multi-line string. tomlkit 0.15.1 started rejecting that, so the command crashed —
and since
tomlkitwas unpinned, CI picked up the new release and thebuildcheckwent red on
mainand therefore on every open PR.This writes each hint line as its own comment instead, which tomlkit is happy with,
and pins
tomlkitso a future release can't break us silently again.What was broken
Item.comment()now validates its argument and raisesValueError("Comment cannot contain line breaks")on any\n(tomlkit/items.py:504).Two call sites passed multi-line strings, so every call to
create_comfynode_config()raised —
comfy node initwas broken at runtime, not just in tests.Reproduced on clean
origin/main@ a732f5c with tomlkit 0.15.1: 9 failed (8 intest_config_parser.py, 1 intest_node_init.py), matching the report exactly.What changed
_add_commented_hint()emits a hint one standalonetomlkit.comment()per line.The helper takes the block and splits it, so the hint text stays verbatim and the
shape that caused the bug is now structurally impossible to reintroduce.
create_comfynode_config.initialize_project_configreparses that file and only rewrites values, so this is the only point where the hint
can be placed under
licenseand before the[project.urls]sub-table.tomlkit>=0.13,<0.16(defense-in-depth, as the ticket suggested).uncommenting actually parses.
Rendered output is byte-identical to the pre-break version except two dropped
trailing spaces and one blank line (artifacts of the old inline attachment). Verified by
diffing this branch's
comfy node initoutput againstorigin/main's under tomlkit 0.13.2.Judgment calls (flagging for review)
I did not use the fix the ticket proposed, because it doesn't hold up. The ticket
suggested
includes.comment('"requires-comfyui" = ...'). That renders inline:includes = [] # "requires-comfyui" = ...— but the stated intent (and the code's owncomment, "uncommentable hint") is a hint beneath the field that a user can uncomment
in place. Inline, it can't be. A standalone comment preserves the original behavior
exactly; I went with that.
The ticket's root cause was incomplete — there is a second call site. It named only
config_parser.py:74-76, but that one is increate_comfynode_config; the 9 failingtests actually die at line 250 in
initialize_project_config, a 17-line commented-outclassifiersblock. Fixing only the reported lines would have left CI red. Both are fixed.Placement is load-bearing, and the obvious fix is wrong. Naively appending the
classifiers comment to the
projecttable renders it after[project.urls]— souncommenting it would define
classifiersunder[project.urls], the wrong table. Thetest asserts the ordering; I verified it fails against that mutation.
Verified across the whole pinned range, not just the version I had installed — the
affected tests pass under tomlkit 0.13.0 / 0.13.2 / 0.14.0 / 0.15.1. (At 0.13.0 blank
hint lines render
#vs#; cosmetic only, and the tests are not overfit to it.)Left alone (pre-existing, out of scope): the
# Used by Comfy Registrycommentrenders inside
[project.urls]rather than at the top of the file. Same class ofplacement bug, but it predates this change — I confirmed identical output on
origin/main— so it isn't part of this fix. Happy to file it separately.
Did not regenerate
uv.lock.uv lockwants to pull in 286 lines of unrelated churn(main's lock is already stale w.r.t. the
benchextra). CI installs viapip install -e .and doesn't enforce the lock, so this is a no-op for the build; keeping the diff scoped.
Verification
ruff check .— passruff format --check .— pass (235 files)pytest— 2575 passed, 37 skipped (the 9 previously-failing tests now green)initialize_project_config()in a scratch git repo and diffed the realemitted
pyproject.tomlagainstorigin/main's.